-
-
Notifications
You must be signed in to change notification settings - Fork 693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Configurable proxy blacklist #774
Conversation
This LGTM (looks great to me). @meeber thoughts? |
* User configurable property, defines which non-existing properties should be ignored | ||
* instead of throwing an error if they do not exist on the assertion. | ||
* This is only applied if the environment Chai is running into supports proxies and | ||
* if the `useProxy` configuration setting is enabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple nits:
- Line 78: Can remove "non-existing" since it's redundant with "if they do not exist on the assertion" later on in the same sentence.
- Line 80: "running in" instead of "running into".
@lucasfcosta Awesome work, thanks for doing this! I just had a few nits related to comments. I really like how you handled the changes, particularly the tests with the helper function. You've done the world a great service by fixing that errant backtick. |
9b2163f
to
46c24d8
Compare
Thanks for the help @meeber, since I'm not a native English speaker I make this kind of mistakes sometimes. Let me know if I forgot anything. |
* if the `useProxy` configuration setting is enabled. | ||
* By default, `then` and `inspect` will not throw an error if they do not exist on the | ||
* assertion object because we should not throw errors on Symbol properties such as | ||
* `Symbol.toStringTag` nor on `.then` because it is necessary for for promise type-checking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The part about Symbol properties and Symbol.toStringTag
can remain in proxify.js because that's where the type check is; it's different than the configurable excluded keys. A new note needs to be added here about .inspect
; it's ignored by default because that property is read by util.inspect
, for example when using console.log
on the assertion object.
@lucasfcosta I'm jealous of your ability to speak multiple languages! I added a follow-up note above about the comments. |
ca2a6d4
to
01220ce
Compare
01220ce
to
05c37fe
Compare
@meeber thanks for noticing that! |
LGTM! |
This aims to close #765.
As brilliantly suggested by @meeber this adds a new Array containing keys which should be ignored when checking for non-existing properties on an assertion before throwing an error.
This PR contains:
backtick
which should have been an'
chai.config
calledproxyExcludedKeys
which receives an array of Strings representing the properties which should be ignored by the proxy on environments that support itthen
andinspect
propertiesthen
andinspect
properties if they get removed from theproxyExcludedKeys
array on environments that do support proxiesLet me know if I missed anything.